Improve index performance on Entity Framework persisters - #5881
Conversation
9798bf1 to
3b359b9
Compare
rbev
changed the title
johnsimons
left a comment
johnsimons
left a comment
There was a problem hiding this comment.
just a couple of comments to consider
| // ingestion rule. EnclosedMessageTypes' first comma token is a type's full name, so the cap can | ||
| // only ever bite on pathological generic names, where a truncated sort key still sorts and | ||
| // groups consistently. | ||
| static string? TruncateForColumn(string? value) => |
There was a problem hiding this comment.
Should the truncate remove the first parts?
In other words, given the type name includes first the namespace, would it make sense to prioritise the class name, hence we start tirmming from left to right?
There was a problem hiding this comment.
I'd say a 450char type name is extremely unlikely, it's almost double the windows file path limit, but yes that's probably a better choice.
| public string Id { get; set; } = null!; | ||
| public string Type { get; set; } = null!; |
There was a problem hiding this comment.
should these be marked as required?
warwickschroeder
left a comment
warwickschroeder
left a comment
There was a problem hiding this comment.
Please double check this:
On PostgreSQL the new sort indexes never serve the failed messages page:
What goes wrong: FilterByStatus builds includes.Contains(message.Status) (src/ServiceControl.Persistence.EFCore/Infrastructure/FailedMessageQueryFilters.cs:42). SQL Server renders a one-element list as [Status] = @includes1, which lets the index hand rows back already sorted. Npgsql renders it as status = ANY (@includes); the planner cannot know the array holds one value, so it cannot assume sorted output and falls back to scanning and sorting.
Fix: when includes.Count == 1, filter with message.Status == includes[0]. The filter shape predates the PR, but the PR's benefit depends on it.
On PostgreSQL the groups query uses neither widened index and is no faster
What goes wrong: SQL Server secondary indexes silently carry the clustered key, which here is UniqueMessageId on FailedMessages and (FailedMessageUniqueId, GroupId) on FailedMessageGroups, so the widened indexes hold the join keys. PostgreSQL indexes point at table rows instead, so unique_message_id and failed_message_unique_id are in neither index and the join must read the tables.
Fix: add the join keys to the PostgreSQL INCLUDE lists and re-measure
3 participants
This pull request introduces new and optimized database indexes to both the SQL Server and PostgreSQL EF Core persistence layers, aiming to improve the performance of queries on failed messages and failure groups. The changes include adding covering indexes (with INCLUDE columns) for key aggregate queries, introducing new indexes to support sorting and filtering in the UI, and enforcing indexable column lengths for message types. These enhancements are reflected in both the migration files and the EF Core model configuration.
Database Indexing Improvements
Added covering indexes (with INCLUDE columns) to the
Status, LastModifiedindex onFailedMessagesand theType, GroupIdindex onFailedMessageGroupsin both SQL Server and PostgreSQL providers, improving index-only scan performance for group aggregate queries. [1] [2] [3] [4] [5] [6] [7] [8]Added new indexes to
FailedMessagesfor(Status, LastTimeOfFailure)and(Status, MessageType, UniqueMessageId)to support efficient sorting and filtering in the failed messages UI. [1] [2] [3] [4] [5] [6] [7]Schema and Model Changes
MessageTypecolumn onFailedMessagesto have a maximum length of 450 characters (from unlimited text) to allow indexing and prevent issues on SQL Server and PostgreSQL. [1] [2] [3] [4] [5]Documentation and Comments
Query Logic Update
GroupsDataStoreto use the new optimized method for fetching unresolved groups, aligning with the new index structures.These changes collectively improve query performance for key recoverability and failed message scenarios, and ensure the schema is optimized for both SQL Server and PostgreSQL backends.